Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EZP-30344: Allowed limiting Content management to specific translations #2585

Merged

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented Mar 25, 2019

Question Answer
JIRA issue EZP-30344
Required by ezsystems/ezplatform-admin-ui#934
Bug/Improvement yes
New feature yes
Target version 7.5.1 for eZ Platform 2.5.1 LTS
BC breaks yes, minor (see JIRA issue)
Tests pass yes
Doc needed yes

Summary

This PR:

  • Improves Language Limitation by limiting user's ability to create, edit and publish content in a given language to a list of translations set by that Limitation.
  • e6e5aa0 implements PermissionResolver::lookupLimitations which provides more standardized and API-oriented way to fetch information about existing Limitations for the given object in the context of the given policy (by @mikadamczyk).

Squashed into one PR due to time constraints.

Implementation details

Many Limitations throw exception when dealing with unknown objects or targets, which makes it impossible to set some specific mixes of Limitations on a given policy. This had hit us in the past and now makes current changes impossible to implement.

To solve this issue we're introducing two new interfaces:

  • \eZ\Publish\SPI\Limitation\Target which is a marker for a new Target object which is designed in a new way and provides more information about an intent of a canUser check.
  • \eZ\Publish\SPI\Limitation\TargetAwareType for LimitationType (service which implements Limitation evaluation logic) which does not throw exception when dealing with an unknown object. Such LimitationType should at the end return ACCESS_ABSTAIN. TargetAwareType LimitationType expects $targets to be an array of instances of Target, however if different object is passed, it won't throw exception (behaves as described).

We've created the first class which objects follows the described behavior:

  • \eZ\Publish\SPI\Limitation\Target\Version

and a builder for DX and readability

  • \eZ\Publish\SPI\Limitation\Target\Builder\VersionBuilder
    which provides behavioral, intent-driven API.

Consider the following updateContent use case:

        $this->repository->getPermissionResolver()->canUser(
            'content',
            'edit',
            $content,
            [
                (new Target\Builder\VersionBuilder())
                    ->updateFieldsTo(
                        $contentUpdateStruct->initialLanguageCode,
                        $contentUpdateStruct->fields
                    )
                    ->build(),
            ]
        )

it checks if a user is able to edit the Content if new Version would contain new initial language and given fields (actually used only to extract language codes of modified translation).

Limitation\Target\Version is passed as one of the targets to the following ContentService methods:

  • createContentDraft // from existing Version
  • updateContent
  • publishVersion // not needed

We could try handling ContentUpdateStruct, but this Target is also designed to provide Limitation lookup for which creating artificial update struct is an overkill.

To provide BC for existing Limitations other than Language and avoid known issues when mixing Limitations for a single policy, the logic of PermissionResolver::canUser is changed as follows:

  • for LimitationTypes not implementing Limitation\TargetAwareType only targets which are not instances of Limitation\Target are passed (array is filtered prior passing). If an array is empty, null is returned as some Limitations rely on that.
  • for Target-aware Limitation types (implementing Limitation\TargetAwareType) only targets which are instances of Limitation\Target are passed.

TODO:

  • Cover COTF requirements mix (Translation, Content type, Parent Location) with integration tests.
  • Implement Languages diff to determine if publishing should be allowed.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@alongosz alongosz force-pushed the ezp-30344-language-limited-content-mgm branch 4 times, most recently from e15470c to b1d2838 Compare April 3, 2019 14:00
@alongosz alongosz changed the title EZP-30344: Implemented Translation Limitation EZP-30344: Improved Language Limitation Apr 4, 2019
@alongosz alongosz changed the title EZP-30344: Improved Language Limitation EZP-30344: Allowed limiting Content management to specific translations Apr 4, 2019
@alongosz alongosz force-pushed the ezp-30344-language-limited-content-mgm branch from b1d2838 to 01a240a Compare April 4, 2019 15:46
@alongosz
Copy link
Member Author

alongosz commented Apr 5, 2019

@mikadamczyk refactored to use VersionBuilder according to @adamwojs request. For changed usages please see 6217d1568 (hint: see extra build method there).

@alongosz alongosz force-pushed the ezp-30344-language-limited-content-mgm branch from 9e61496 to 15cd9b6 Compare April 9, 2019 22:00
@alongosz
Copy link
Member Author

Fixed code review issues and added more flexibility to Target\Version via a9b7c2d for better permission lookup.

@micszo micszo self-assigned this Apr 11, 2019
alongosz and others added 8 commits April 12, 2019 13:25
Improved implementation takes care about a target
which describes future intended changes to a Content item
Instance of Target provides to TargetAwareLimitationType better target
which describes the actual intent which cannot be inferred from
a content item itself

changed canUser invocation for:
* createContentDraft
* updateContent
* publishVersion
@alongosz alongosz force-pushed the ezp-30344-language-limited-content-mgm branch from c67e9d3 to f0c45ea Compare April 12, 2019 11:25
Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@micszo micszo removed their assignment Apr 17, 2019
@alongosz alongosz merged commit 0f847fd into ezsystems:master Apr 18, 2019
@alongosz alongosz deleted the ezp-30344-language-limited-content-mgm branch April 18, 2019 15:10
@alongosz alongosz mentioned this pull request Oct 30, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants